Codecov Report

Attention: Patch coverage is 88.60759% with 9 lines in your changes missing coverage. Please review.

Project coverage is 91.78%. Comparing base (2a0c23a) to head (4546bea).
Report is 70 commits behind head on main.

@@            Coverage Diff             @@
##             main     #711      +/-   ##
==========================================
- Coverage   91.80%   91.78%   -0.03%     
==========================================
  Files          45       45              
  Lines        6959     7045      +86     
==========================================
+ Hits         6389     6466      +77     
- Misses        570      579       +9     

Thanks for the PR.

• before merging we need to update the format version; we probably should add a format for the SpatialData container; something asked also by several users (we already have a format for the single elements, which would be independent from the format of the container).

Hi @LucaMarconato, have you started working on your last comment? If not, do you want me to have a look (although I'm not exactly sure to understand what you want to do)?

I noticed we are missing a feature: after we save a SpatialData object, if we add a new attr, there is currently no way to save it on disk. Something like sdata.write_element for the attrs

Just a random note on this: I agree that having SpatialData - level attributes would be super helpful. Beyond that, I think it would be great to be able to annotate coordinate systems - these often correspond to samples, conditions, time-points etc. I can of course annotate all elements within a coordinate system through tables, but I think it would be more direct to somehow directly provide metadata for an entire coordinate system.

I also noticed that we want to pass the attrs inside certain functions, e.g. when performing a query (the attrs should be kept). I made a commit for this

The only concern is when we concatenate multiple SpatialData objects, how should we merge the duplicate keys (if any)?

NB: I updated the first post with a check-list

@quentinblampey thanks for the update. I haven't updated the format yet, we can work on it this week at the hackathon. Regarding saving to disk, I suggest to extend the write_metadata() method. Finally, regarding .attrs, we could proceed as AnnData does for .uns when calling ad.concat().

Extremely interested in this, thanks @quentinblampey !!

@quentinblampey I have finalized this PR, I'm going to merge it now. Thanks for your contribution!

Two comments.

1. root-level versioning: On disk, I am now adding a version number for the root level metadata, and also I am adding the string of the spatialdata software version that was used to create the metadata (this helps reproducibility). When reading the .zattrs from disk into the SpatialData.attrs slow I am not parsing the two version strings not to clutter the object.
2. .attrs passed by reference: I have removed the code dict(sdata.attrs) and passing the attrs always by reference. The only exceptions are when I call spatialdata.deepcopy(), where I call copy.deepcopy(), and when the user passes a Mapping that is not a dict. In such case the casting to dict performs a shallow copy.